feat(mbp): recovery consolidate from wallet address #52

cpatino-intive · 2025-07-01T01:38:10Z

recovery consolidate from wallet address
TASKS: WP-5143

Copilot

Pull Request Overview

Adds support for recovering consolidation sweeps from TRX and Solana wallet addresses via a new /recoveryConsolidations endpoint.

Extend coinUtils to recognize Sol and TRX coins.
Define a new API spec, router entry, handler, and client interfaces for the recovery consolidations endpoint.
Add end‐to‐end tests covering TRX and Solana consolidation recovery flows.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/shared/coinUtils.ts	Added `isSolCoin` and `isTrxCoin` helpers.
src/api/master/routers/masterApiSpec.ts	Declared new request/response types and route spec.
src/api/master/handlers/recoveryConsolidationsWallet.ts	Implemented handler logic for TRX/Sol consolidation recovery.
src/api/master/clients/enclavedExpressClient.ts	Extended `RecoveryMultisigOptions` to accept MPC transaction types.
src/tests/api/master/recoveryConsolidationsWallet.test.ts	Added tests for TRX and Solana consolidation recovery.

Comments suppressed due to low confidence (1)

src/api/master/routers/masterApiSpec.ts:189

Using t.any for the signed transactions response is too permissive. Consider defining a more specific transaction schema to improve validation and client typing.

    signedTxs: t.array(t.any), // Array of fully signed transactions

src/api/master/handlers/recoveryConsolidationsWallet.ts

src/api/master/routers/masterApiSpec.ts

src/api/master/handlers/recoveryConsolidationsWallet.ts

pranavjain97 · 2025-07-02T22:20:20Z

src/api/master/handlers/recoveryConsolidationsWallet.ts

+  );
+
+  // 1. Build unsigned consolidations
+  if (isSolCoin(sdkCoin)) {


No need for having these if conditions. If the recoverConsolidations method is implemented for a coin, we should continue. If not, then we should throw an error saying it is not supported.

pranavjain97 · 2025-07-02T22:21:04Z

src/api/master/handlers/recoveryConsolidationsWallet.ts

+    txs = result.transactions;
+  }
+
+  console.log(txs);


Use logger.debug

pranavjain97 · 2025-07-02T22:21:47Z

src/api/master/handlers/recoveryConsolidationsWallet.ts

+  const signedTxs = [];
+  try {
+    for (const tx of txs) {


why are there multiple unsigned sweeps?

The return result from sdkCoin.recoverConsolidations is transaction array.

I guess they are all txs from receive address -> base.

pranavjain97 · 2025-07-02T22:23:13Z

src/api/master/handlers/recoveryConsolidationsWallet.ts

+      userKey: userPub,
+      backupKey: backupPub,
+      bitgoKey,
+      durableNonces: req.decoded.durableNonces!,


instead of doing this; just do an if check to throw an error if coin is sol and req.decoded.durableNonces is undefined. But call sdkCoin.recoverConsolidations( only once

The base branch was changed.

mohammadalfaiyazbitgo · 2025-07-08T18:50:53Z

src/api/master/handlers/recoveryConsolidationsWallet.ts

+  const signedTxs = [];
+  try {
+    for (const tx of txs) {


I guess they are all txs from receive address -> base.

mohammadalfaiyazbitgo · 2025-07-08T18:51:52Z

src/api/master/handlers/recoveryConsolidationsWallet.ts

+  const result = await (sdkCoin as any).recoverConsolidations({
+    ...req.decoded,
+    userKey: userPub,
+    backupKey: backupPub,
+    bitgoKey,
+    durableNonces: req.decoded.durableNonces,
+  });


don't cast to any, use the appropriate coin classes that support recoverConsolidations

mohammadalfaiyazbitgo · 2025-07-08T18:53:13Z

src/api/master/handlers/recoveryConsolidationsWallet.ts

+  if (typeof (sdkCoin as any).recoverConsolidations !== 'function') {
+    throw new Error(`recoverConsolidations is not supported for coin: ${coin}`);
+  }


same here. create a type guard instead that validates sdkCoin supports recoverConsolidations and returns an extended BaseCoin type.

mohammadalfaiyazbitgo · 2025-07-08T18:53:32Z

src/api/master/routers/masterApiSpec.ts

+  durableNonces: t.union([
+    t.undefined,
+    t.type({
+      publicKeys: t.array(t.string),
+      secretKey: t.string,
+    }),
+  ]),


nit: can use optional

mohammadalfaiyazbitgo · 2025-07-08T18:53:46Z

src/api/master/routers/masterApiSpec.ts

+  startingScanIndex: t.union([t.undefined, t.number]), // Optional
+  endingScanIndex: t.union([t.undefined, t.number]), // Optional
+  seed: t.union([t.undefined, t.string]), // Optional (Sol)
+  tokenContractAddress: t.union([t.undefined, t.string]), // Optional (TRX/Sol)


can just use optional(t.x)

mohammadalfaiyazbitgo · 2025-07-08T18:54:09Z

src/api/master/routers/masterApiSpec.ts

+  userPub: t.string,
+  backupPub: t.string,
+  bitgoKey: t.string,


I don't think you need all 3 values, the pub for them all is going to be the same.

Ticket: WP-5166

mohammadalfaiyazbitgo · 2025-07-16T02:07:17Z

src/api/master/handlers/recoveryConsolidationsWallet.ts

+    bitgoKey: bitgoPub,
+  });
+
+  console.log(`Recovery consolidations result: ${JSON.stringify(result)}`);


don't think we need this.

cpatino-intive force-pushed the WP-5143/recovery-consolidate branch from e45c55c to f4f0859 Compare July 1, 2025 01:45

cpatino-intive requested review from Copilot, mohammadalfaiyazbitgo and pranavjain97 July 1, 2025 16:32

Copilot AI reviewed Jul 1, 2025

View reviewed changes

pranavjain97 requested changes Jul 2, 2025

View reviewed changes

cpatino-intive force-pushed the WP-5143/recovery-consolidate branch from f679941 to 8aff26f Compare July 3, 2025 19:01

cpatino-intive requested a review from pranavjain97 July 3, 2025 19:16

cpatino-intive changed the base branch from master to WP-4736/utxo-recoveries July 3, 2025 19:17

cpatino-intive changed the base branch from WP-4736/utxo-recoveries to WP-4793/utxo-accelerate July 3, 2025 19:18

pranavjain97 previously approved these changes Jul 4, 2025

View reviewed changes

Base automatically changed from WP-4793/utxo-accelerate to master July 7, 2025 11:30

mohammadalfaiyazbitgo requested changes Jul 8, 2025

View reviewed changes

cpatino-intive force-pushed the WP-5143/recovery-consolidate branch 3 times, most recently from acaef72 to 5675ff4 Compare July 11, 2025 17:47

cpatino-intive changed the base branch from master to WP-5166-eddsa-recovery-v2 July 11, 2025 17:47

mohammadalfaiyazbitgo force-pushed the WP-5166-eddsa-recovery-v2 branch 3 times, most recently from ff6ecf8 to 88eac6b Compare July 11, 2025 20:05

cpatino-intive force-pushed the WP-5143/recovery-consolidate branch from 5675ff4 to 41f7b16 Compare July 14, 2025 13:36

Base automatically changed from WP-5166-eddsa-recovery-v2 to master July 14, 2025 22:14

mohammadalfaiyazbitgo and others added 6 commits July 15, 2025 19:46

feat: eddsa recover txn signature

e908fe6

Ticket: WP-5166

feat(mmbe): recovery consolidation

ca90869

feat(mbe): fix rebase

cb6757d

feat(mbe): fix rebase

e3a288e

feat(mbe); fix recovery consolidation for eddsa

22d49dc

feat(mbe): add integration test

57775b3

cpatino-intive force-pushed the WP-5143/recovery-consolidate branch from 75b87bc to 57775b3 Compare July 15, 2025 23:49

feat(mbe): fix liniting

d356c9a

cpatino-intive requested review from mohammadalfaiyazbitgo and pranavjain97 July 16, 2025 01:56

mohammadalfaiyazbitgo approved these changes Jul 16, 2025

View reviewed changes

mohammadalfaiyazbitgo merged commit 0e5ab4f into master Jul 16, 2025
3 checks passed

mohammadalfaiyazbitgo deleted the WP-5143/recovery-consolidate branch July 16, 2025 19:12

feat(mbp): recovery consolidate from wallet address #52

feat(mbp): recovery consolidate from wallet address #52

Uh oh!

Conversation

cpatino-intive commented Jul 1, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants